Support setting Error.stackTraceLimit automatically #419
Conversation
|
🎱 |
mattrobenolt
force-pushed
the
stacktracelimit
branch
from
0523247 to
89f0f5c
Compare
|
@benvinegar I rebased this against master since there were a bunch of conflicts. Wanna take a look now? |
|
I guess a thing to consider, even with POST, is we don't have room for infinite payloads. We only have 100KB to send for the POST body. So in retrospect, not sure if this is a great idea to go to Infinity. Maybe just increase it to 50? |
| @@ -168,6 +170,7 @@ Raven.prototype = { | |||
| this._isRavenInstalled = true; | |||
| } | |||
|
|
|||
| Error.stackTraceLimit = this._globalOptions.stackTraceLimit; | |||
There was a problem hiding this comment.
Do we really need to modify this? Why is it not sufficient to just slice frames before transmitting?
Note this means that multiple error instances of Raven.js will clobber each other's Error.stackTraceLimit.
There was a problem hiding this comment.
re-reads PR message
Oh, I see re: V8. Hmm.
There was a problem hiding this comment.
Yeah, this only affects V8, and unfortunately, it's a global option.
V8 just defaults to a limiting, 10 frames. We've personally hit an issue with this in Sentry since React generates some long stack traces.
In theory, we'd push all browsers to capture Infinity frames, then slice() out of that, but that's not how reality works. :(
In V8, `Error.stack` is truncated with a default `Error.stackTraceLimit` of 10. So we want to expose this and set it to Infinity to capture it all.
We don't have infinite room to send data, we only have 100KB, even for POST, so we need to be a little bit conservative. 50 should be enough for most cases. If we need to handle more, we'll need to implement compression on the client so our packet is less than 100KB like our other server side clients.
mattrobenolt
force-pushed
the
stacktracelimit
branch
from
145e3a9 to
5871cc1
Compare
Support setting Error.stackTraceLimit automatically
In V8,
Error.stackis truncated with a defaultError.stackTraceLimitof 10. So we want to expose this and set it to 50 to capture more.
/cc @dcramer
Also worth noting, that this is probably useless without XHR being the default transport, since this will very easily push us over our querystring limitations.